fix: "ERR_INVALID_THIS" on Node 20#86
Conversation
Gozala
left a comment
There was a problem hiding this comment.
Please note that formatting and other inconsistencies in the fetch sub-package were due to it been forked from node-fetch that used particular style. I thought it would be easier to pull upstream changes this way.
That said I have not really pulled any changes from node-fetch so perhaps that decision is worth revisiting. Given you've put more time into this than myself, you should make call what makes more sense going forward. I have no objections either way
|
@MichaelDeBoey looks like this breaks some tests, could you please fix them before landing this ? |
| return URLSearchParams.prototype[p].call( | ||
| receiver, | ||
| target, | ||
| String(name).toLowerCase() |
There was a problem hiding this comment.
Looks like this introduces a regression in tests see https://github.com/web-std/io/actions/runs/5986735537/job/16286188008?pr=86
There was a problem hiding this comment.
@mcansh Any idea what this could have caused?
There was a problem hiding this comment.
narrowed it down to internal version mismatches between packages so installing/building the packages weren't using the latest changes
edit: tested with pnpm and workspace:* the packages together
There was a problem hiding this comment.
26f3294 to
a5a297b
Compare
9c7b1db to
126988a
Compare
0ef14d2 to
64fdbb4
Compare
* fix: node 20 pnpm/node-fetch#1 Signed-off-by: Logan McAnsh <logan@mcan.sh> * ci: test against more node versions Signed-off-by: Logan McAnsh <logan@mcan.sh> * ci: update action versions Signed-off-by: Logan McAnsh <logan@mcan.sh> * chore: remove nested .github dir Signed-off-by: Logan McAnsh <logan@mcan.sh> * ci: disable fail fast Signed-off-by: Logan McAnsh <logan@mcan.sh> * ci: new server per test Signed-off-by: Logan McAnsh <logan@mcan.sh> * test: update family agent option test Signed-off-by: Logan McAnsh <logan@mcan.sh> * Create polite-eggs-jam.md * test: use ipv4 Signed-off-by: Logan McAnsh <logan@mcan.sh> * test: use family 0 Signed-off-by: Logan McAnsh <logan@mcan.sh> --------- Signed-off-by: Logan McAnsh <logan@mcan.sh>
Signed-off-by: Logan McAnsh <logan@mcan.sh>
792ae4b to
c5f7708
Compare
There was a problem hiding this comment.
Copilot reviewed 15 out of 24 changed files in this pull request and generated 1 comment.
Files not reviewed (9)
- package.json: Language not supported
- packages/blob/package.json: Language not supported
- packages/fetch/.github/FUNDING.yml: Language not supported
- packages/fetch/.github/ISSUE_TEMPLATE/config.yml: Language not supported
- packages/fetch/.github/workflows/ci.yml: Language not supported
- packages/fetch/.github/workflows/commonjs.yml: Language not supported
- packages/fetch/.github/workflows/lint.yml: Language not supported
- packages/fetch/.github/workflows/types.yml: Language not supported
- packages/fetch/package.json: Language not supported
Comments suppressed due to low confidence (6)
packages/fetch/src/headers.js:141
- Verify that using 'target' in place of 'receiver' is appropriate for this call to maintain consistent behavior across header operations.
target,
packages/fetch/.github/PULL_REQUEST_TEMPLATE.md:1
- [nitpick] The removal of the pull request template may be unintended; please confirm if this change aligns with the team's workflow expectations.
<file removed>
packages/fetch/.github/ISSUE_TEMPLATE/support-or-usage.md:1
- [nitpick] The removal of the support or usage issue template should be verified to ensure it was a deliberate decision and not an accidental deletion.
<file removed>
packages/fetch/.github/ISSUE_TEMPLATE/feature-request.md:1
- [nitpick] Please confirm that the removal of the feature request issue template was intentional and that it does not disrupt the expected issue reporting process.
<file removed>
packages/fetch/.github/ISSUE_TEMPLATE/bug_report.md:1
- [nitpick] Ensure that removing the bug report issue template aligns with your team's reporting requirements and that an alternative is in place if needed.
<file removed>
.github/workflows/fetch.yml:72
- [nitpick] The Coveralls step has been removed from the workflow. Confirm that this removal is intentional and that an alternative coverage reporting solution is in place if needed.
# upload coverage only once
See @mcansh's remix-run#32
CC/ @alanshaw @Gozala